Fix TIFF setup responsiveness and ingestion follow-up#15
Fix TIFF setup responsiveness and ingestion follow-up#15AdvancedImagingUTSW merged 2 commits intomainfrom
Conversation
- add a zarr3-safe TIFF Dask loader with memmap fallback so TIFF reads continue to work after the zarr v3 migration - reuse the TIFF helper in ingestion paths and cover the fallback with targeted IO regression tests - switch setup-stage TIFF metadata verification to tifffile.TiffFile header parsing so file selection no longer constructs expensive Dask arrays - cache per-experiment setup metadata/source resolution in the GUI and reuse cached context when preparing stores on Next - add Delete/Backspace handling for setup-list removal and document the updated GUI behavior - document the actual runtime ingestion execution model, including that TIFF/HDF-backed writes are Dask-parallel but currently execute through the local threaded scheduler rather than the distributed client path
- apply Black formatting to the Python files touched by the TIFF ingestion/setup follow-up work - keep the formatter-only changes isolated in a separate commit after the functional fix set
There was a problem hiding this comment.
Pull request overview
This PR improves TIFF handling and setup responsiveness by adding a zarr v3–safe Dask TIFF loader with fallbacks, switching Navigate TIFF setup metadata reads to header-only parsing, and caching per-experiment metadata/source resolution in the GUI. It also adds keyboard-based list removal in the setup dialog and documents the ingestion execution model.
Changes:
- Add
open_tiff_as_dask()with a zarr v3 compatibility fallback path, and update TIFF ingestion to use it. - Speed up setup-stage TIFF metadata verification via
tifffile.TiffFile(...)header parsing and add GUI caching for per-experiment metadata/source resolution. - Add Delete/Backspace removal in the setup list and expand runtime docs describing ingestion execution behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/clearex/io/read.py |
Introduces open_tiff_as_dask() and routes TIFF Dask loads through it. |
src/clearex/io/experiment.py |
Adds TIFF header-only metadata parsing for Navigate setup flow; switches TIFF Dask source opening to open_tiff_as_dask(). |
src/clearex/gui/app.py |
Adds per-experiment metadata cache, cache invalidation, and Delete/Backspace list removal via eventFilter. |
src/clearex/gui/README.md |
Documents new setup-list key removal and metadata caching behavior. |
docs/source/runtime/ingestion-and-canonical-store.rst |
Documents the ingestion/materialization execution model and scheduler distinctions. |
tests/io/test_read.py |
Adds regression test covering TIFF Dask fallback when da.from_zarr rejects ZarrTiffStore. |
tests/io/test_experiment.py |
Adds tests ensuring TIFF header-only setup metadata read and TIFF Dask fallback at experiment layer. |
tests/gui/test_gui_execution.py |
Adds test ensuring Delete key removes selected experiment entry. |
src/clearex/io/provenance.py |
Formatting-only change to _default_outputs. |
src/clearex/workflow.py |
Formatting-only change to a ValueError raise. |
src/clearex/flatfield/pipeline.py |
Formatting-only change to weighted accumulation expressions. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| try: | ||
| return ( | ||
| da.from_zarr(tiff_store, chunks=chunks) | ||
| if chunks is not None | ||
| else da.from_zarr(tiff_store) | ||
| ) | ||
| except TypeError as exc: | ||
| if "Unsupported type for store_like" not in str(exc): |
There was a problem hiding this comment.
open_tiff_as_dask detects the zarr3 incompatibility by matching a substring in the TypeError message ("Unsupported type for store_like"). This is brittle across Dask/Zarr versions and locales; a small upstream message change would skip the fallback. Prefer using a structural check (e.g., store type/attributes) or a more explicit exception contract to decide when to fall back.
| try: | |
| return ( | |
| da.from_zarr(tiff_store, chunks=chunks) | |
| if chunks is not None | |
| else da.from_zarr(tiff_store) | |
| ) | |
| except TypeError as exc: | |
| if "Unsupported type for store_like" not in str(exc): | |
| # Detect the specific tifffile/Zarr3 incompatibility structurally rather | |
| # than by matching exception message text. | |
| ZarrTiffStore = getattr(tifffile, "ZarrTiffStore", None) | |
| is_zarr_tiff_store = ( | |
| ZarrTiffStore is not None and isinstance(tiff_store, ZarrTiffStore) | |
| ) | |
| try: | |
| zarr_version = str(getattr(zarr, "__version__", "0")) | |
| zarr_major = int(zarr_version.split(".", 1)[0]) | |
| except (ValueError, TypeError): | |
| zarr_major = 0 | |
| is_zarr3 = zarr_major >= 3 | |
| try: | |
| return ( | |
| da.from_zarr(tiff_store, chunks=chunks) | |
| if chunks is not None | |
| else da.from_zarr(tiff_store) | |
| ) | |
| except TypeError as exc: | |
| # Only fall back for the known ZarrTiffStore/Zarr3 incompatibility; | |
| # propagate other TypeErrors unchanged. | |
| if not (is_zarr_tiff_store and is_zarr3): |
| except Exception: | ||
| logger.info( | ||
| "tifffile.memmap unavailable for %s; using tifffile.imread fallback.", | ||
| path.name, |
There was a problem hiding this comment.
The except Exception: around tifffile.memmap(...) discards the original exception details. This makes it hard to diagnose why memmap was unavailable (unsupported dtype, bad kwargs, permissions, etc.) and can hide real regressions. Capture the exception and include it in the log (e.g., via exc_info), and consider narrowing the caught exception types if possible.
| except Exception: | |
| logger.info( | |
| "tifffile.memmap unavailable for %s; using tifffile.imread fallback.", | |
| path.name, | |
| except Exception as exc: | |
| logger.info( | |
| "tifffile.memmap unavailable for %s; using tifffile.imread fallback.", | |
| path.name, | |
| exc_info=True, |
| if prefer_dask: | ||
| # Option A: use tifffile's OME-as-zarr path if possible | ||
| # This keeps it lazy and chunked without loading into RAM | ||
| store = tifffile.imread(str(path), aszarr=True) | ||
| darr = da.from_zarr(store, chunks=chunks) if chunks else da.from_zarr(store) | ||
| darr = open_tiff_as_dask(path, chunks=chunks, **kwargs) | ||
| info = ImageInfo( |
There was a problem hiding this comment.
TiffReader.open documents that **kwargs are passed through to tifffile.imread, but currently only the Dask path forwards **kwargs (via open_tiff_as_dask). The NumPy branch still calls tifffile.imread(str(path)) without **kwargs, which makes the API behavior inconsistent depending on prefer_dask. Either forward **kwargs in the non-Dask branch too, or adjust the docstring/parameters to reflect the actual supported behavior.
| if isinstance(description_payload, dict): | ||
| metadata.update(description_payload) | ||
| spacing = description_payload.get("spacing") | ||
| unit = str(description_payload.get("unit", "")).strip().lower() |
There was a problem hiding this comment.
TIFF header parsing only treats units in { "um", "micron", "microns" } as microns. OME/TIFF metadata commonly uses the micro sign ("µm") (and sometimes Greek mu "μm"), which will currently be ignored and prevent voxel_size_um_zyx from being populated even when spacing is present. Consider accepting these additional unit spellings when normalizing unit.
| unit = str(description_payload.get("unit", "")).strip().lower() | |
| raw_unit = str(description_payload.get("unit", "")).strip() | |
| unit = raw_unit.replace("µ", "u").replace("μ", "u").lower() |
Summary
tifffile.TiffFile(...)and cache per-experiment metadata/source resolution so file selection is much fasterValidation
uv run black --check src/clearex/flatfield/pipeline.py src/clearex/gui/app.py src/clearex/io/experiment.py src/clearex/io/provenance.py src/clearex/io/read.py src/clearex/workflow.pyuv run ruff check src/clearex/flatfield/pipeline.py src/clearex/gui/app.py src/clearex/io/experiment.py src/clearex/io/provenance.py src/clearex/io/read.py src/clearex/workflow.py tests/gui/test_gui_execution.py tests/io/test_experiment.py tests/io/test_read.pyuv run python -m py_compile src/clearex/gui/app.py src/clearex/io/experiment.py src/clearex/io/read.pyuv run --with pytest --with requests python -m pytest -q tests/io/test_read.py -k "open_tiff_as_dask_falls_back_when_store_is_unsupported"uv run --with pytest --with requests python -m pytest -q tests/io/test_experiment.py -k "open_source_as_dask_tiff_falls_back_when_store_is_unsupported or load_navigate_experiment_source_image_info_reads_tiff_header_only"uv run --with pytest --with requests python -m pytest -q tests/gui/test_gui_execution.py -k "delete_key_removes_selected_experiment"